Skip to content

Conversation

pavoljuhas
Copy link
Collaborator

@pavoljuhas pavoljuhas commented Sep 5, 2025

  • Delegate SingleQubitPauliStringGateOperation.__mul__, __rmul__ to
    its PauliString base
  • Put back faster to-PauliString conversion of Pauli gates

Follow-up to #7603
Related to #7588

…ring base

SingleQubitPauliStringGateOperation inherits from GateOperation and
PauliString in that resolution order, i.e., with `__mul__`, `__rmul__`
defaulting to those of GateOperation.  To avoid infinite recursion
we make it use the PauliString operations instead.

Also avoid creating a new operation in `_try_interpret_as_pauli_string`
if the argument is SingleQubitPauliStringGateOperation which is already
a PauliString.
@pavoljuhas pavoljuhas requested review from a team and vtomole as code owners September 5, 2025 01:43
@github-actions github-actions bot added the size: M 50< lines changed <250 label Sep 5, 2025
@pavoljuhas
Copy link
Collaborator Author

NB: Remove the t7603 directory before merging this in.

Copy link

codecov bot commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.56%. Comparing base (c35ed72) to head (bdd9d63).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7637   +/-   ##
=======================================
  Coverage   99.56%   99.56%           
=======================================
  Files        1088     1088           
  Lines       96048    96062   +14     
=======================================
+ Hits        95632    95646   +14     
  Misses        416      416           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pavoljuhas
Copy link
Collaborator Author

PR #7603 made multiplications with Paulis more general and correct, but it also introduced slow down for multiplication with simple Paulis.
I have added a temporary timing script and results to the directory t7603 to benchmark multiplications before and after #7603 and at this PR. #7603 caused about 2-fold slow down, after this PR the timings are either similar or better than before:

# all times in μs with stddev-s in parentheses
# x = X(q(0)), x1 = X(q(1)), x2 = X(q(2))

x**2 * x * x * x  72(3)   --> 169(5)   --> 48(2)
x**1 * x**4       40(1)   --> 88(2)    --> 42(3)
x1 * x2           21.8(2) --> 78(2)    --> 13(1)

@pavoljuhas pavoljuhas requested a review from daxfohl September 5, 2025 02:09
@pavoljuhas pavoljuhas changed the title Optimize multiplication of Pauli operations Optimize multiplications with Pauli operations Sep 5, 2025
Copy link
Collaborator

@daxfohl daxfohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 I was wondering about the perf impact of these changes. One other change in that PR that could be a regression is the new self.on(*qubits) in Gate, which creates and initializes a new operation. If _try_interpret_as_pauli_string had an optional qubits parameter, you could just pass the gate and the qubits as-is without having to create a new operation.

@pavoljuhas
Copy link
Collaborator Author

pavoljuhas commented Sep 5, 2025

+1 I was wondering about the perf impact of these changes. One other change in that PR that could be a regression is the new self.on(*qubits) in Gate, which creates and initializes a new operation.

That is needed, because GateOperation.__mul__ is done by Gate._mul_with_qubits which does not accept Operation. It seems all callers of _mul_with_qubits have an Operation object, so perhaps we could replace this with Gate._mul_operation which would not need to use Gate.on(). That would be for some other PR and we'd need some benchmarks first to make sure it is worthwhile to do.

@github-actions github-actions bot added size: S 10< lines changed <50 and removed size: M 50< lines changed <250 labels Sep 8, 2025
@pavoljuhas pavoljuhas enabled auto-merge September 8, 2025 17:43
@pavoljuhas pavoljuhas added this pull request to the merge queue Sep 8, 2025
Merged via the queue into quantumlib:main with commit ba8f55a Sep 8, 2025
35 checks passed
@pavoljuhas pavoljuhas deleted the optimize-paulis-multiplication branch September 8, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants